feat(lk): project resolution refactor, other UX#850
Conversation
Topherhindman
left a comment
Topherhindman
left a comment
There was a problem hiding this comment.
couple nits, but overall definitely a good improvement!
|
|
||
| // poll for keys | ||
| fmt.Printf("Please confirm access by visiting:\n\n %s\n\n", authURL.String()) | ||
| out.Statusf("Please confirm access by visiting:\n\n %s\n", authURL.String()) |
There was a problem hiding this comment.
out.Statusf is --quiet-suppressed, so lk cloud auth -q i think would hide the verification URL, right?
There was a problem hiding this comment.
Correct, it doesn't print the URL -- but it does still open a browser window automatically if currently on an interactive terminal.
| p.Warnf("warn %d", 1) | ||
| p.Resultf("result\n") | ||
|
|
||
| assert.Empty(t, "", err.String()[:0], "sanity") |
There was a problem hiding this comment.
is this actually testing anything?
| } | ||
| if !silent { | ||
| fmt.Printf("Creating new agent deployment\n") | ||
| out.Status("Creating new agent deployment") |
There was a problem hiding this comment.
--silent and --quiet overlap here. these out.Status* calls already respect --quiet internally, but they're also wrapped in if !silent, so they're double gated, where as here its only gated by --quiet.
should we fold --silent's status suppression into --quiet and drop the if !silent wrappers?
There was a problem hiding this comment.
Yes, good call. I'll just collapse these into one flag (--quiet). The cool thing about this is we almost don't need it at all, cuz now that everything happens on the expected FD you can do cmd > /dev/null 2>&1 to mimic silent, or cmd 2>/dev/null to suppress statuses.
There was a problem hiding this comment.
Still nice to have the flag for it tho
2 participants
Printerstruct to make it easier to do file redirection, respect--quietand--verboseflags globally, and to keep output from getting mixed in with debug information. Status messages and command outputs should not be printed manually withfmtfunctions from now on, and should instead use e.g.out.Status(),out.Result(), etc.BEFORE:
AFTER: